Percent-decode username and password parsed from URLs (#1871) - #2114

I think this is a fine idea; but I worry that some who have "%" in their username/pw will not get errors and have to add the extra step to encode first.

The @Jeansidharta reproducer is exactly this
case: paramiko is handed pass%23with%23hash%23char and rejects it.

Can you explain this a bit more, please?

The @Jeansidharta reproducer is exactly this
case: paramiko is handed pass%23with%23hash%23char and rejects it.

Can you explain this a bit more, please?

Hi @martindurant. I believe the original comment this quote came from should clarify. In short, I was trying to connect to a SFTP server using a password that contains a # character, using fsspect.open_files with a URL, like this:

fsspec.open_files("sftp://username:pass#with#hash#char@ftpserver.com/path")

But this fails, as the # character in the password is treated as a fragment separator. I then tried quoting my password:

from urllib.parse import quote

fsspec.open_files(f"sftp://username:{quote("pass#with#hash#char")}@ftpserver.com/path")

But, unfortunately, urllib.urlsplit does not unquote the netloc part, and fsspec does not handle this case either. My password was sent as pass%23with%23hash%23char directly to paramiko, which caused an auth error. I believe it is currently impossible to connect to an sftp server with this password using just the URL. I had to pass these values through kwargs:

fsspec.open_files("sftp://ftpserver.com/path", username="username", password="pass#with#hash#char")

This is a valid workaround for sftp, but is not guaranteed to work with all backends, as they may not use the username and password kwargs (examples: WebHDFS uses user, and GithubFileSystm uses token instead of password)

As has been mentioned, this change would be a silently breaking change for some users; the worst kind of change. Maybe it could be implemented as an optional argument to open_files and open, such as unquote_url, or maybe as a different function altogether?

@martindurant — good point, and I agree the silent-mismatch case is worth softening. The residual risk is a user whose password contains a bare % followed by two hex digits: urlsplit accepts that (urllib never enforces RFC-3986 escaping in the netloc), but unquote on the default errors='replace' would decode it and, when the byte is not valid UTF-8, produce a U+FFFD instead of an error.

Pushed 90befa0 to route both fields through a small _unquote_userinfo helper that uses errors='strict' and falls back to the raw string on UnicodeDecodeError:

def _unquote_userinfo(value: str) -> str:
    try:
        return unquote(value, errors="strict")
    except UnicodeDecodeError:
        return value

Net behavior on realistic passwords:

The only remaining ambiguity is the very last row — a user whose raw password literally is 50%25off (four printable chars including %, 2, 5) has always been in the position of needing to double-encode it in a URL. That's inherent to using RFC-3986 percent-encoding for anything at all, and there's no local information here that can distinguish it from someone who correctly encoded 50%off as 50%25off.

Regression test extended to cover 50%off, pass%, and pass%ab alongside the encoded case, and the changelog entry now names the fallback. Rebased onto master while I was in there.

I'm not sure about that. Wouldn't you say: the string is either encoded or not?

Fair point, and you're right in principle: per RFC 3986 the userinfo is percent-encoded, so a % in a password should always arrive as %25 and the string is encoded, full stop. My per-escape idea invents a third mode that is neither, and it would be hard to document.

So the question is only what to do with URLs that were never encoded, which is what the fallback exists for. Decoding unconditionally is the clean rule, and it does change behaviour for anyone passing 50%off today and meaning it literally. If that is acceptable, drop the fallback and the _unquote_userinfo helper with it, call unquote(...) on both fields, and say in the docstring that userinfo must be encoded; the pass%ab case then becomes a genuine error rather than something to paper over.

If that compatibility is worth keeping, the current whole-value fallback is the better of the two shims, since "it decodes, or it is left exactly as it came" is at least a rule you can state in one line. Either way I withdraw the per-escape suggestion.

The compatibility is worth keeping

A comment is fine if it is short!

Reworded the comment above _unquote_userinfo (233195c) to name the all-or-nothing behavior explicitly — same 5 lines, no functional change:

def _unquote_userinfo(value: str) -> str:
    # Percent-decode a URL userinfo component (username or password).
    # The fallback is deliberately all-or-nothing: the value is either decoded
    # in full or kept exactly as it arrived, so a literal ``%`` followed by two
    # hex digits that happens to produce a non-UTF-8 byte (e.g. a password
    # containing ``%ab``) is preserved intact instead of being replaced by
    # U+FFFD, and no caller ever sees a partially-decoded string.
    try:
        return unquote(value, errors="strict")
    except UnicodeDecodeError:
        return value

The three-case test matrix (%23/%40/%c3%a9 decode, 50%off/pass%/pass%ab preserve) still pins the behavior; the comment just makes the intent hard to miss.

I was hoping for a short comment: one line only